Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve "infrastructure" crate wide #88

Merged
merged 9 commits into from
Apr 3, 2023

Conversation

tcharding
Copy link
Member

@tcharding tcharding commented Mar 2, 2023

Improve various things around the crate (infrastructure for want of a better word)

Please note: Includes bump of MSRV to 1.48.0 because arrayvec was failing with 1.41 after improving CI.

EDIT: Oooooh, now I get it, arrayvec feature breaks the MSRV, regardless of if its 1.41 or 1.48 - I forgot that. I"m totally sick of rebasing this so I'm leaving the MSRV bump in there.

  • CI
  • github actions
  • linting
  • docs
  • githooks

Patch 7 ef35d89 Enable build of docs with doc_auto_cfg fixes #75

@tcharding tcharding mentioned this pull request Mar 3, 2023
@tcharding
Copy link
Member Author

Please do not merge until #45 is done so @Ericson2314 does not have to rebase on top of this.

@tcharding tcharding force-pushed the 03-03-infra branch 3 times, most recently from 8560399 to 8096aff Compare March 6, 2023 00:39
@tcharding tcharding force-pushed the 03-03-infra branch 2 times, most recently from e203ed6 to 04c59a8 Compare March 21, 2023 23:39
@tcharding
Copy link
Member Author

Checked over the whole patch series, made minor improvements removing an unrelated change from one patch.

Added an additional patch to do a clippy fix.

git diff-index --check --cached $against -- || exit 1

# Check that code lints cleanly.
cargo clippy --all-features -- -D warnings || exit 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a cool addition. Confirmed working after opting in to hooks.

Copy link
Member Author

@tcharding tcharding Mar 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah the git hooks are super useful, it often pains me to wait each time I commit so I tend to use -n a bit which ignores the hooks i.e., git commit -a -m 'WIP: dirty commit' -n

@apoelstra
Copy link
Member

I guess the docsrs thing should be changed now to use doc_auto_cfg or whatever that was.

Move the license to a separate file and use an SPDX identifier in the
source file. This is more terse but with the same legal coverage.

Keep Clark's name as author but add "the rust-bitcoin developers" also
for attribution and accountability.
@tcharding tcharding force-pushed the 03-03-infra branch 3 times, most recently from b1a4026 to 313b1e2 Compare April 3, 2023 02:23
Add whitespace, and use second layer markdown header for subheading.
@tcharding tcharding force-pushed the 03-03-infra branch 2 times, most recently from 28deec8 to e60d87c Compare April 3, 2023 03:49
As we are doing throughout the rust-bitcoin ecosystem; bump the MSRV to
Rust 1.48.0
We do not have any unknown lints, I'm not even sure its possible to get
them now that we have the MSRV in the clippy config file.
Clippy emits:

 warning: trait objects without an explicit `dyn` are deprecated

We are currently configuring the linter to allow this but it is not
necessary.

Use `dyn` as suggested by the linter and remove
`allow(bare_trait_objects)`.
Currently we are setting a bunch of lint config options to their
default values, this is redundant.
Use the recently learned about `doc_auto_cfg` feature and enable it
using a custom `docsrs` compiler conditional configuration option.

Fix and improve the rustdocs while we are at it.

Add docs build to CI script but do not enable it in the github actions
yet.

Fix: rust-bitcoin#75
Improve the CI pipeline by doing:

- Run docs build with stable/nightly as required
- Run 32 bit tests (default features with a 32 bit target)
- Run cross tests (default features with a s390x target)
Add a `githooks` directory and a `pre-commit` hook. Add a section to the
readme instructing devs how to configure git to take advantage of the
githooks.
@apoelstra
Copy link
Member

ooo TIL about git config --local core.hooksPath githooks/ I may actually start using your hooks now that I don't have to manually copy them into .git/hooks..

Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK c691a83

@apoelstra apoelstra merged commit 8f6d5d9 into rust-bitcoin:master Apr 3, 2023
@apoelstra apoelstra deleted the 03-03-infra branch April 3, 2023 13:48
@apoelstra apoelstra mentioned this pull request Apr 3, 2023
@Ericson2314
Copy link
Contributor

LGMT!

@tcharding
Copy link
Member Author

ooo TIL about git config --local core.hooksPath githooks/ I may actually start using your hooks now that I don't have to manually copy them into .git/hooks..

lolz we merged that command into various readmes so long ago :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document features
4 participants